Feature/bca/verif resist no age#6328
Conversation
BillCarsonFr
force-pushed
the
feature/bca/verif_resist_no_age
branch
2 times, most recently
from
111869d to
6cc1fce
Compare
BillCarsonFr
requested review from
a team,
fedrunov and
mnaturel
and removed request for
a team
BillCarsonFr
force-pushed
the
feature/bca/verif_resist_no_age
branch
from
d858ed7 to
9929d6a
Compare
|
related to matrix-org/synapse#8429 |
BillCarsonFr
added
Z-NextRelease
bmarty
left a comment
bmarty
left a comment
There was a problem hiding this comment.
Some remarks, since this is "NextRelease", I have made a review. Did not test the code though.
...android/src/main/java/org/matrix/android/sdk/internal/database/helper/ThreadSummaryHelper.kt
Outdated
Show resolved
Hide resolved
...n/java/im/vector/app/features/crypto/verification/choose/VerificationChooseMethodFragment.kt
Outdated
Show resolved
Hide resolved
...n/java/im/vector/app/features/crypto/verification/choose/VerificationChooseMethodFragment.kt
Outdated
Show resolved
Hide resolved
.../java/im/vector/app/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt
Outdated
Show resolved
Hide resolved
| android:layout_height="wrap_content" | ||
| android:gravity="center_horizontal" | ||
| android:orientation="horizontal" | ||
| tools:visibility="visible"> |
There was a problem hiding this comment.
tools:visibility can be removed.
vector/src/main/res/layout/item_positive_destrutive_buttons.xml
Outdated
Show resolved
Hide resolved
.../java/im/vector/app/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt
Outdated
Show resolved
Hide resolved
mnaturel
left a comment
mnaturel
left a comment
There was a problem hiding this comment.
Good for me though I didn't correctly understand how to test and if is possible to test. I tried triggering some session verifications but I have not the same UI with simple "Accept"/"Decline" bottom sheet. So I am not sure this is the correct tests.
Not related, but I simply noticed for a while the top bar notification to trigger a verification is not displayed just after the first sign in. You need to restart the app to see it, I don't know if it was an intended change.
|
SonarCloud Quality Gate failed. |
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginEnd="16dp" | ||
| tools:text="@string/action_decline" /> |
There was a problem hiding this comment.
OK to use tools but in this case, I think that positiveText and destructiveText should not be nullable. (see https://github.com/vector-im/element-android/pull/6328/files#diff-e03d15165cd3cc55c597e31b0494b0088b8ef56da1edb7468b870973d2948265R35)
This is not a blocker though.
3 participants
Type of change
Content
Under certain condition, incoming verification are displayed in the UI without Accept/Decline button.
Making it impossible to continue the verification.
This PR fixes the case when a verification request is ignored because deemed too in the past or too in the future.
This computation is based on ageLocalTs, that is computed from the
unsigned.age. The processor and the timeline display had different test, the timeline was accepting null age and not processor.To fix it, we now ensure that
ageLocalTsis set even if nounsigned.agein the synced event.I noticed that it was also possible to open the bottom sheet from typing on the verification tile and not on one of the button. I modified the bottom sheet to show accept/decline buttons when the request is not yet ready.
Motivation and context
Screenshots / GIFs
Tests
Tested against polyjuice for regressions.
Tested devices
Checklist